Skip to content

[Performance] Allocate AlignedTVList primitive arrays lazily#18276

Merged
jt2594838 merged 4 commits into
masterfrom
allocate_primitive_array_lazily_pr
Jul 23, 2026
Merged

[Performance] Allocate AlignedTVList primitive arrays lazily#18276
jt2594838 merged 4 commits into
masterfrom
allocate_primitive_array_lazily_pr

Conversation

@jt2594838

Copy link
Copy Markdown
Contributor

Description

Follow-up to #18264. This PR contains only the lazy primitive-array allocation changes on top of the merged dynamic-bitmap implementation.

  • Keep null placeholders for newly added or unwritten aligned columns and allocate each PrimitiveArray only on its first actual write.
  • Update TsFileProcessor memory accounting for lazy primitive-array allocation.
  • Cover clone/read/WAL/null-tablet paths with unit tests.
  • Add a 256 MiB integration test that writes 40,000 rows to one aligned measurement and then one row to 1,200 new measurements, preventing the eager-allocation memory failure.
  • Add a real 1C1D integration performance comparison against a master worktree, including both sparse/lazy-column and fully non-null aligned-tablet workloads, and generate a Markdown report.

Performance

Compared master 91476b0df7 with candidate b7f846ec98:

Workload master candidate Result
Sparse/lazy columns 30.598 ms 14.294 ms latency -53.29%, throughput +114.07%
Fully non-null aligned tablets 123.767 ms 123.359 ms +0.33%

Tests

  • AlignedTVList unit tests: 14/14 passed.
  • 256 MiB low-memory integration test passed (40,000 historical rows, then one row across 1,200 new columns).
  • 1C1D worktree comparison completed for master and candidate, including the no-null workload.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calculateRamSize() still multiplies the number of timestamp blocks by alignedTvListArrayMemCost(), which assumes that every column has a materialized primitive array in every block. That assumption is no longer valid after this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in a335107. AlignedTVList now updates materializedValueArrayMemCost when getOrCreateValueArray() allocates a primitive array. calculateRamSize() uses the per-block cost without value primitive arrays plus the incrementally tracked materialized-array cost, so it no longer assumes dense value arrays or scans all blocks. The tracked state is also handled by clone, projection, column extension, and clear paths.

Comment on lines 1145 to 1150

if (acquireArray != 0) {
// memory of extending the TVList
memIncrements[0] +=
acquireArray * alignedMemChunk.getWorkingTVList().alignedTvListArrayMemCost();
for (TSDataType dataType : dataTypesInTVList) {
memIncrements[0] += acquireArray * AlignedTVList.valueListArrayMemCost(dataType);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still charges the dense per-block cost for every existing column whenever a new block is acquired, while AlignedTVList.expandValues() now only adds null placeholders and allocates primitive arrays on their first actual write.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in a335107. The aligned row, rows, and tablet estimators now split the charge into: (1) the base aligned-TVList block cost without value primitive arrays, (2) placeholder/bitmap cost for newly extended columns, and (3) primitive-array cost only when a successful non-null write first materializes that column/block. Sparse, null, and failed-write paths are covered by tests; the tablet path also skips per-row checks when both bitmap and results are absent.

@Caideyipi

Copy link
Copy Markdown
Collaborator

Added regression coverage in commit 27e985663b5 for the two memory-accounting issues noted above.

Both tests intentionally fail on the current PR head:

TsFileProcessorTest#testAlignedSparseRowDoesNotChargeUnallocatedPrimitiveArrayOnNewBlock
expected:<272> but was:<0>

AlignedTVListTest#testCalculateRamSizeExcludesUnallocatedPrimitiveArrays
expected:<2384> but was:<3440>

The second mismatch is 1056 bytes, exactly two unallocated INT64 primitive arrays (2 blocks x 528 bytes). The first shows that a sparse row and a dense row crossing the same block boundary are currently charged the same increment, although the dense row materializes one additional INT32 primitive array (272 bytes).

Besides calculateRamSize(), the dense per-block charge appears in all three write paths in TsFileProcessor: single-row around line 852, InsertRows around line 951, and Tablet around line 1149. The regression tests should turn green once those paths charge only primitive arrays that are actually materialized.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

@jt2594838
jt2594838 merged commit 02b1ecd into master Jul 23, 2026
41 of 43 checks passed
@jt2594838
jt2594838 deleted the allocate_primitive_array_lazily_pr branch July 23, 2026 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants